feat(accounts-controller): add support for Snap keyring v2#8513
feat(accounts-controller): add support for Snap keyring v2#8513
Conversation
ccharly
force-pushed
the
cc/feat/accounts-controller-snap-keyring-v2
branch
from
2851f32 to
235c68e
Compare
| const generatePatch = (): StatePatch => { | ||
| return { | ||
| previous: {}, | ||
| added: [], | ||
| updated: [], | ||
| removed: [], | ||
| }; | ||
| }; | ||
| const patches = { | ||
| snap: generatePatch(), | ||
| normal: generatePatch(), | ||
| }; | ||
|
|
||
| // Gets the patch object based on the keyring type (since Snap accounts and other accounts | ||
| // are handled differently). | ||
| const patchOf = (type: string): StatePatch => { | ||
| if (isSnapKeyringType(type)) { | ||
| return patches.snap; | ||
| } | ||
| return patches.normal; |
There was a problem hiding this comment.
Just removed all this since we don't need that separation at all!
| metadata: { | ||
| name: '', | ||
| importTime: Date.now(), | ||
| lastSelected: 0, | ||
| keyring: { | ||
| type: KeyringType.Snap, | ||
| }, | ||
| snap: { | ||
| name: keyring.snapId, | ||
| enabled: true, | ||
| id: keyring.snapId, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The new Snap keyring v2 only use KeyringAccount, thus we don't have any metadata for those!
| snap: { | ||
| name: keyring.snapId, | ||
| enabled: true, | ||
| id: keyring.snapId, | ||
| }, |
There was a problem hiding this comment.
We could query this from the SnapController, but that requires a new allowed action, thus a breaking change (but at least that would mimic what the current big SnapKeyring is doing)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c9d2191. Configure here.
| ...account, | ||
| // We still have to use internal account for now, so we inject some metadata. | ||
| metadata: { | ||
| name: '', |
There was a problem hiding this comment.
Snap v2 accounts permanently stuck with empty name
Medium Severity
#getAccountFromSnapKeyringV2 hardcodes metadata.name to ''. When the stateChange handler creates a v2 account, this empty name gets persisted. Later, updateAccounts attempts to assign a default name using existingAccount?.metadata.name ?? 'Snap Account X', but the nullish coalescing operator (??) does not treat empty strings as nullish, so '' ?? 'Snap Account 1' evaluates to ''. The account name can never be corrected to a proper default.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c9d2191. Configure here.
There was a problem hiding this comment.
That's fine, we're in te process of deprecating account names on those objects anyway.
1 participant
Explanation
Adding support for Snap keyring v2 for this controller. For now, those keyrings are not in use, but we will need this to be able to the accounts associated with those new keyrings.
References
N/A
Checklist
Note
Medium Risk
Adds new account ingestion path for
SnapKeyringv2 during keyring state sync andupdateAccounts, which could affect which accounts are created/removed if keyring types or lookup behavior differ. Changes are covered by new tests for v2 add/skip edge cases, keeping overall risk moderate.Overview
Adds support for
SnapKeyringv2 accounts soAccountsControllercan materialize Snap v2 keyring addresses into internal accounts by iterating v2 keyring instances and usinglookupByAddress, injecting backward-compatible snap metadata.Updates keyring-type utilities to recognize
KeyringType.Snap(v2) as a Snap keyring (naming and filtering), and extends tests to cover v2 add/update flows and edge cases (missing keyring instance, account deleted before lookup). Also updates the messenger CLI scripts to use theoxfmtformatter and records the new capability in the changelog.Reviewed by Cursor Bugbot for commit 3caea20. Bugbot is set up for automated code reviews on this repo. Configure here.